Skip to content

Run tests in CI#37

Closed
nettle wants to merge 1 commit intomainfrom
00031-github-clang
Closed

Run tests in CI#37
nettle wants to merge 1 commit intomainfrom
00031-github-clang

Conversation

@nettle
Copy link
Copy Markdown
Collaborator

@nettle nettle commented Jul 30, 2025 •
edited
Loading

Why: we have GitHub action but it does not run any tests

What:

  • Set Clang version (append particular llvm version to PATH)
  • Check tools (which and versions)
  • Run Bazel test
  • Run unit tests
  • Remove --stats (not supported by open-source CodeChecker)
  • Change extension to yaml

@nettle
Run tests in CI
22f5dcb 
Why: we have GitHub action but it does not run any tests

What:
- Set Clang version
- Check tools (which and versions)
- Run Bazel test
- Run unit tests
- Remove --stats (not supported by open-source CodeChecker)
- Change extension to yaml
@nettle nettle force-pushed the 00031-github-clang branch from 230cbc7 to 22f5dcb Compare July 30, 2025 15:33
@nettle nettle mentioned this pull request Jul 30, 2025
Merged
@nettle nettle requested review from Szelethus, dkrupp and furtib July 30, 2025 15:41
@Szelethus Szelethus mentioned this pull request Jul 30, 2025
Merged
Szelethus
Szelethus requested changes Jul 31, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally strongly in favour of letting the original author finish the original work and learn in the process. It's fine to submit totally alterantive solutions to the same problem, we gained a lot from doing that in #21, but not quite the same solution with marginal differences.

GITHUB_PATH is interesting, but doesn't align with the, unfortunately only spoken, agreed on goals. The goal as I understand was to keep everything as simple as possible, representative of how a new user/contributor would set up their environment, which this GitHub Workflow specific solution doesn't achieve.

I think this should've been discussed as an idea in #28 and it is what I'd encourage moving forward.

@nettle
Copy link
Copy Markdown
Collaborator Author

nettle commented Jul 31, 2025

I'm personally strongly in favour of letting the original author finish the original work and learn in the process.

That's totally fine with me. I'm not insisting on anything, just showing alternative approaches.

GITHUB_PATH is interesting, but doesn't align with the, unfortunately only spoken, agreed on goals. The goal as I understand was to keep everything as simple as possible, representative of how a new user/contributor would set up their environment, which this GitHub Workflow specific solution doesn't achieve.

Well, as I understand GITHUB_PATH is a standard GitHub Action approach (Adding a system path), which is equivalent to export PATH=/usr/lib/llvm-18/bin:$PATH. So, to me personally it looks a way simpler and much more flexible. But again, I'm not insisting.

I think this should've been discussed as an idea in #28 and it is what I'd encourage moving forward.

Also fine with me.

@Szelethus
Copy link
Copy Markdown
Contributor

Szelethus commented Aug 7, 2025

Landed in #28.

@Szelethus Szelethus closed this Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Szelethus Szelethus Szelethus requested changes

@dkrupp dkrupp Awaiting requested review from dkrupp

@furtib furtib Awaiting requested review from furtib

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nettle @Szelethus